feat: Add tizen support #680
Conversation
further testing will be needed
controls and colors are still not great. more work needs done
created a specific video_player_controls for the tizen. Can possibly be used for all TVs. It is mostly just a copy of the already existing DesktopOverlay.
there was an issue where there was added an extra / in the transcode url. not sure why. I've removed it in a imo not so great way, but is, for now, fixes the issue
small other fixes and clean up of code
reused the videoWidget from lib_mkd, which is correctly setup and thereby fixed the issue. corrected other small things
Added tizen_video_hole to be able to scale the video player independant of the UI scaling of the app. Also removed TrueHD from tizen_profile because only some TrueHD seem to be working
also updated version number
popdollar
force-pushed
the
feat/add-tizen-support
branch
from
3dcc76d to
d8e7737
Compare
There was a problem hiding this comment.
Sorry for the late reply. And thank you for having a crack at getting Fladder to Samsung TV's.
I've left a few comments. Let me know if any of them are unclear.
The basic idea is we need to separate some of the calls that are needed for Tizen in new files. And create stub files for the other platforms to call.
There are probably some things I missed but let's fix the current comments and we can see where we go from there.
| ); | ||
| } | ||
|
|
||
| static LazyDatabase _openConnectionTizen() { |
There was a problem hiding this comment.
What is the reason we need to open the database this way for tizen?
Are downloads even supported for on samsung tv's?
There was a problem hiding this comment.
We can probably leave this out completely, or do we need to check for resolution changes on Samsung TV's?
There was a problem hiding this comment.
This file could use some clean-up. I'm also not a big fan of the way navigation is handled, we seem to be hard-coding a navigation path.
It's fine to leave it as is for now if it works. But will probably update this with a more rigid like a FocusTraversalGroup solution.
The native Flutter controls have to be updated for dPad navigation at some point any way.
Do remove the commented out code if it's no longer need though.
| } | ||
|
|
||
| @override | ||
| Future<void> pause() => _controller?.pause() ?? Future.value(); |
There was a problem hiding this comment.
| Future<void> pause() => _controller?.pause() ?? Future.value(); | |
| Future<void> pause() async => _controller?.pause(); |
Let's do this instead of returning a empty Future.value();
| @override | ||
| Future<void> pause() => _controller?.pause() ?? Future.value(); | ||
| @override | ||
| Future<void> play() => _controller?.play() ?? Future.value(); |
There was a problem hiding this comment.
| Future<void> play() => _controller?.play() ?? Future.value(); | |
| Future<void> play() async => _controller?.play(); |
Same here.
| Future<int> setAudioTrack(AudioStreamModel? model, PlaybackModel playbackModel) async { | ||
| final wantedAudioStream = model ?? playbackModel.defaultAudioStream; | ||
| if (wantedAudioStream == AudioStreamModel.no() || wantedAudioStream == null) { | ||
| return -1; |
There was a problem hiding this comment.
This no longer allows for disabling audio. Is that something not supported by the Tizen player?
There was a problem hiding this comment.
This file is probably the biggest "pain" point for merging this into the main repository. It also seems to be missing some imports but that could be because I'm using the default flutter sdk.
If we want to keep everything in a single repo my suggestion would be to rename this file to tizen_player.dart.orig. And create a new "stub" file named tizen_player.dart that extends the BasePlayer so we don't get any errors.
That way when working on Tizen we can simply swap the file out for the original and continue from there.
| return DynamicColorBuilder( | ||
| builder: (ColorScheme? lightDynamic, ColorScheme? darkDynamic) { | ||
| final lightTheme = themeColor == null | ||
| final rawLightTheme = themeColor == null |
There was a problem hiding this comment.
Do we need this change?
There was a problem hiding this comment.
Probably best to create a new "tizen" only pubspec that we can then swap between when we need to debug/build tizen.
| } | ||
|
|
||
| void main(List<String> args) async { | ||
| if (isTizen) { |
There was a problem hiding this comment.
Logic like this should be moved to a new file. So we can create a "stub" file like mentioned previously for the other platforms that implements the functions/boolean with nothing attached.
That also includes the following imports.
import 'dart:ffi' as ffi;
import 'package:flutter_tizen/flutter_tizen.dart';
import 'package:sqlite3/open.dart' as sqlite3_open;
While only the ffi is not supported on some platforms best to not use it at all by default if possible.
2 participants
Pull Request Description
Added support Tizen OS 6.0+ for Samsung TVs. It is in a working state (at least on my TV S90C and the emulator).
It is build with Samsungs Flutter SDK.
I've used tizens videohole video_player since they recommend using it when specifically targeting TVs.
Disclaimer
I've targeted main functionality and there might be changes that are breaking, since this have not been tested with any other OS. I don't believe the additional packages specifically for Tizen are breaking for any other clients, but I can not guarantee anything.
Checklist